Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update slash_in_parameter.rst #4307

Closed
wants to merge 1 commit into from
Closed

Update slash_in_parameter.rst #4307

wants to merge 1 commit into from

Conversation

SamanShafigh
Copy link
Contributor

"name" in requirements is confusing, I had problem for doing the same and then after 2 days I found that the "name" should be same as the parameter, it is not part of symfony config

requirements:
name: .+

"name" in requirements is confusing, I had problem for doing the same and then after 2 days I found that the "name" should be same as the parameter, it is not part of symfony config

   requirements: 
        name: .+
@xabbuh
Copy link
Member

xabbuh commented Oct 9, 2014

@SamanShafigh Thank you for improving the documentation. However, I'm mixed on this. What do others think? Is the name name really confusing? Anyway, I would then prefer to write username instead of userName.

@weaverryan Should then be merged into 2.3.

@wouterj
Copy link
Member

wouterj commented Oct 9, 2014

@SamanShafigh what exactly did you find confusing about "name" and how does "username" solve that? (I don't really follow you, so I would like to know more to see if we can fix it)

@SamanShafigh
Copy link
Contributor Author

Hi,

Thanks for the response,

The problem for the 'name' is that one might think that the "name" in this
line "requirements: name: .+" is part of the symphony rout configs. For
example I did this and I had a problem for several days

cms_show_page:
path: /page/{pageUrl}
defaults: { _controller: SamanCmsBundle:Page:index }
requirements:
name: .+

and then after many searches I found that it should be

cms_show_page:
path: /page/{pageUrl}
defaults: { _controller: SamanCmsBundle:Page:index }
requirements:
pageUrl: .+

On Thu, Oct 9, 2014 at 9:15 PM, Wouter J [email protected] wrote:

@SamanShafigh https://github.com/SamanShafigh what exactly did you find
confusing about "name" and how does "username" solve that? (I don't really
follow you, so I would like to know more to see if we can fix it)


Reply to this email directly or view it on GitHub
#4307 (comment).

@xabbuh
Copy link
Member

xabbuh commented Oct 10, 2014

I'm not sure if many people struggle with this. But I can see that you might misunderstand that name good be a generic identifier in the routing context. Changing name to username would indeed make this absolutely clear. I don't mind if we change it here since it doesn't hurt. So, if it helps people to understand the example, I'd say let's go for it. Let's see what the others think about this.

@wouterj
Copy link
Member

wouterj commented Oct 10, 2014

Ah, so you thought name was some sort of name of the route? I can see how it can be confusing (and wow, thanks for your feedback. This are things we really need feedback from our users, just like you did).

However, can you please change this PR to use username instead of userName? Then we're consistent with the rest of the documentation.

@SamanShafigh
Copy link
Contributor Author

Hi,

Thanks for the update, Sure I will change it to username

Best

Saman

On Fri, Oct 10, 2014 at 7:39 PM, Wouter J [email protected] wrote:

Ah, so you thought name was some sort of name of the route? I can see how
it can be confusing (and wow, thanks for your feedback. This are things we
really need feedback from our users, just like you did).

However, can you please change this PR to use username instead of userName?
Then we're consistent with the rest of the documentation.


Reply to this email directly or view it on GitHub
#4307 (comment).

@wouterj wouterj mentioned this pull request Oct 31, 2014
@wouterj
Copy link
Member

wouterj commented Oct 31, 2014

Thanks for starting this, @SamanShafigh. I've made the final changes and created a new PR: #4400

@wouterj wouterj closed this Oct 31, 2014
weaverryan added a commit that referenced this pull request Nov 20, 2014
This PR was merged into the 2.3 branch.

Discussion
----------

Continues #4307

Continues #4307

| Q   | A
| --- | ---
| Doc fix? | yes
| New docs? | no
| Applies to | all
| Fixed tickets | -

Original PR description:

 > "name" in requirements is confusing, I had problem for doing the same and then after 2 days I found that the "name" should be same as the parameter, it is not part of symfony config

Commits
-------

b412780 Changed userName to username
48f453d Update slash_in_parameter.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants